Fix unordered collection handling and project accessor patterns#178
Merged
Conversation
…DefaultXxxWs (#147) Three sites in allomorph.py used self._obj.OwnerOfClass.project.DefaultVernWs / DefaultAnalWs. OwnerOfClass is an LCM method (requires an Int32 class-id arg); bare attribute access returns the bound method object, so .project raises AttributeError, silently swallowed by the bare except — making every property return "". Fixed by using self._obj.Cache.DefaultVernWs / DefaultAnalWs instead, matching the pattern used by the May-20 sweep in phonological_rule.py et al. Closes #147 https://claude.ai/code/session_01WsYz9YzNkyFzkStEr1y8Fg
…stead of OwnerOfClass.project (#153) clone_properties() tried to resolve project via dest.OwnerOfClass.project. OwnerOfClass is an LCM method (requires an Int32 class-id arg); accessing it as a bare attribute returns the bound method object, not an ICmObject, so .project raises AttributeError -- silently swallowed by the bare except. This left `project = None`, permanently killing the collection-clone branch at line 511 (if project:) for every caller that didn't pass project explicitly. Fix: use dest.Cache.LanguageProject, the canonical accessor used elsewhere in the codebase since commit 611f5a3. Closes #153 https://claude.ai/code/session_01WsYz9YzNkyFzkStEr1y8Fg
… support positional ops (#167) LcmOwningCollection (OC suffix) implements ICollection, not IList. Calling IndexOf or Insert on it either silently no-ops or raises in pythonnet, so insert_after=True on OC-backed Duplicate methods never worked. For the two half-fixed sites (WfiGloss, WfiAnalysis) the list(...).index() workaround avoided the IndexOf failure but the follow-up .Insert() was still broken. Fix: replace the entire insert_after branch with an unconditional .Add() on the six affected OC collections: - NoteOperations.py: AnnotationsOC - DataNotebookOperations.py: RecordsOC - GramCatOperations.py: TypesOC - DiscourseOperations.py: ChartsOC - WfiGlossOperations.py: MeaningsOC - WfiAnalysisOperations.py: AnalysesOC OS-backed collections (SubRecordsOS, RepliesOS, SubPossibilitiesOS) are unaffected -- positional Insert is well-defined there. Closes #167 https://claude.ai/code/session_01WsYz9YzNkyFzkStEr1y8Fg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR corrects two systematic issues across the codebase:
Unordered Collection (OC) Handling: Several
Duplicate()methods were incorrectly attempting to insert duplicated objects at specific positions within unordered collections. Since OC collections don't support positional insertion, theinsert_afterparameter is a no-op for these collections and objects should always be added at the end.Project Accessor Pattern: Updated property access patterns to use the canonical
Cache.LanguageProjectaccessor instead of the indirectOwnerOfClass.projectpattern, which is more reliable and follows LCM best practices.Key Changes
Unordered Collection Fixes
ChartsOCinsertion logic to always useAdd()instead of conditionally usingInsert()AnalysesOCinsertion logic to always useAdd()MeaningsOCinsertion logic to always useAdd()TypesOCinsertion logic to always useAdd()RecordsOCinsertion logic to always useAdd()AnnotationsOCinsertion logic to always useAdd()Project Accessor Updates
self._obj.OwnerOfClass.projecttoself._obj.Cache.DefaultVernWsandself._obj.Cache.DefaultAnalWsfor direct writing system accessclone_properties()to resolve project viadest.Cache.LanguageProjectinstead ofdest.OwnerOfClass.projectImplementation Details
insert_afteris a no-opinsert_afterparameter remains in method signatures for API compatibility but is effectively ignored for unordered collectionshttps://claude.ai/code/session_01WsYz9YzNkyFzkStEr1y8Fg